-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[TypeScript] Support default exports in TSExportAssignment. #1528
[TypeScript] Support default exports in TSExportAssignment. #1528
Conversation
moduleDecl.body.body.forEach((moduleBlockNode) => { | ||
if (exportedDecls.length === 0) { | ||
// Export is not referencing any local declaration, must be re-exporting | ||
m.namespace.set('default', captureDoc(source, docStyleParsers, n)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about n
here. Would it be worth looking through the m.imports
to see if any matches the exported name? We don't do that on regular exports. E.g.:
import { test } from './test';
export { foo }; // all good...
3 similar comments
It should read about |
@JounQin can you elaborate? TS's module system is broken without synthetic imports and esModuleInterop enabled; I'm completely comfortable telling people they need to enable those if they want stuff to work properly. |
What a great PR! I was searching for a solution all day, finally gave up and started to dig into the sources. Strangely enough, this PR didn't come up while I was searching for similar issues. @JounQin I agree with you, in my PR I'm reading from mappers = configPaths!
// turn glob patterns into paths
.reduce<string[]>(
(paths, path) => paths.concat(isGlob(path) ? globSync(path) : path),
[],
)
.map(loadConfig)
.filter(isConfigLoaderSuccessResult)
.map(configLoaderResult => {
const matchPath = createMatchPath(
configLoaderResult.absoluteBaseUrl,
configLoaderResult.paths,
)
return (source: string, file: string) => {
// exclude files that are not part of the config base url
if (!file.includes(configLoaderResult.absoluteBaseUrl)) {
return undefined
}
// look for files based on setup tsconfig "paths"
return matchPath(source, undefined, undefined, extensions)
}
}) That being said, we should either do the right thing and figure out how to handle complex cases. Or, we can do the simple thing and only cover simple case (only root tsconfig) or introduce new leg in Also, I can't disagree with @ljharb about assuming that everyone uses |
c639b5b
to
381267a
Compare
] | ||
const exportedDecls = ast.body.filter(({ type, id, declarations }) => | ||
declTypes.includes(type) && | ||
(id && id.name === exportedName || declarations.find(d => d.id.name === exportedName)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey! Here declarations
is undefined unless type === 'VariableDeclaration'
so it crashes the linter in some cases. I'll create another PR to fix it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Nice fixing 🚀! When is it going to be released? |
Allows TypeScript export assignments of default identifiers and/or mixed merged declarations. E.g.:
Fixes: #1527